Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid invocations of errors.LineTooLong. #1335

Merged
merged 1 commit into from
Nov 1, 2016

Conversation

filmor
Copy link
Contributor

@filmor filmor commented Oct 26, 2016

What do these changes do?

Fix unhelpful messages on LineTooLong exceptions due to invalid or incomplete calls to their constructor.

Are there changes in behavior for the user?

Only in the message that they see in this case.

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new entry to CHANGES.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using #issue_number format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.

@@ -102,7 +102,8 @@ def parse_headers(self, lines):
header_length += len(line)
if header_length > self.max_field_size:
raise errors.LineTooLong(
'limit request headers fields size')
'request header field %s' % bname.decode("utf8"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bname.decode('utf-8') can fail with own exception

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it though? This is caught before by "Invalid Header". Changed it anyhow :)

@@ -113,7 +114,8 @@ def parse_headers(self, lines):
else:
if header_length > self.max_field_size:
raise errors.LineTooLong(
'limit request headers fields size')
'request header field %s' % bname.decode("utf8"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@@ -173,7 +175,7 @@ def __call__(self, out, buf):
raw_data = yield from buf.readuntil(
b'\r\n\r\n', self.max_headers)
except errors.LineLimitExceededParserError as exc:
raise errors.LineTooLong(exc.limit) from None
raise errors.LineTooLong('request header', exc.limit) from None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks inconvenient: LineTooLong above is raised with single argument (formated string) and with two args here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not, and the call here is wrong. Check out the constructor of LineTooLong (didn't change that one), it expects a "reason" and a limit that was exceeded. The way this was passed before resulted in a broken error message like "400 got more than Unknown bytes when reading 32768" when it should have read "400 got more than 32768 when reading request header".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, missed that one.

@filmor
Copy link
Contributor Author

filmor commented Oct 26, 2016

I'll fix the failing tests and add some more.

@filmor filmor changed the title Fix invalid invocations of errors.LineTooLong. [WIP] Fix invalid invocations of errors.LineTooLong. Oct 26, 2016
@filmor filmor force-pushed the fix_line_too_long branch from 1f4d4c7 to f5cab5b Compare October 26, 2016 13:24
@filmor filmor changed the title [WIP] Fix invalid invocations of errors.LineTooLong. Fix invalid invocations of errors.LineTooLong. Oct 26, 2016
@asvetlov asvetlov merged commit f5cab5b into aio-libs:master Nov 1, 2016
@asvetlov
Copy link
Member

asvetlov commented Nov 1, 2016

Thanks!

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants